Fix #15155 FN accessMoved (string concatenation using +=) - #8996

AI review

The str().size() == 2 check that detects compound assignment leaves out <<= and >>=, because those operators are 3 characters long. So x <<= 1 / x >>= 1 on a moved object still gets no accessMoved warning, while x += 1 now does. The same gap makes other value types inconsistent: for example, int x; x += y; now gets an uninitvar warning, but int x; x <<= y; stays on legacyUninitvar. Writing it as tok->astParent()->str() != "=" (or Token::Match(parent, "%assign%") && parent->str() != "=") would cover every compound operator.

I found no other new problems. The full testrunner passes. The new nullPointerArithmetic on p += a with a default argument p = 0 matches what base already reports for p + a, so that change makes the two consistent rather than adding a new kind of false positive.

Testing

Built cppcheck and testrunner in pr/, and cppcheck in base/.

• ./testrunner -q (all tests): 5387 tests, 0 failures.

<<= / >>= are missed (scratch/sh.cpp):

#include <utility>
#include <vector>
struct S { std::vector<int> v; S& operator+=(int); S& operator<<=(int); S& operator>>=(int); };
void s1(S x) { S a{ std::move(x) }; x += 1; }
void s2(S x) { S a{ std::move(x) }; x <<= 1; }
void s3(S x) { S a{ std::move(x) }; x >>= 1; }

cppcheck -q --enable=warning --library=std sh.cpp

• base: nothing
• pr: only sh.cpp:4:37: warning: Access of moved variable 'x'. [accessMoved]. Nothing is reported for lines 5 and 6.

The same inconsistency shows with uninitialized values: in pr, int x; x += y; gives uninitvar, but int x; x <<= y; still gives only legacyUninitvar.

Other checks:

• The PR's case (std::string with += after a move) now gives accessMoved, as intended.
• x = "abc"; x += "d"; and x.clear(); x += "d"; after a move: no false positive.
• void f(int a, int *p = 0) { p += a; }: the new nullPointerArithmetic matches what base already reports for int *q = p + a;.
• Integer x = 0; x += h(y); if (x == 0) and x |= h(y): no new warnings.

Automated review of 5812614 by Claude. The pull request was built and tested locally.

AI review

I found no new problems. The earlier review said that <<= and >>= were missed. The current code checks str().size() > 1 on %assign%, which covers every compound operator, and the new tests check both <<= and +=. In my tests the only new findings were real ones:

• accessMoved on x += ... after a move, including through reference parameters.
• uninitvar replacing legacyUninitvar for +=, *= and ^= on uninitialized scalars and pointers.
• nullPointerArithmeticRedundantCheck for if (!p) {} p += n;. The base version already reports this for the equivalent p + n.

I found no new false positives in the integer, symbolic or condition cases I tried.

Testing

I built cppcheck and testrunner in pr/ and cppcheck in base/.

• ./testrunner -q (all tests): 0 failures.

I compared base and PR with cppcheck -q --enable=all --inconclusive --library=std on my own files in scratch/. The results:

Changed (all correct):

• void f1(std::string x){ std::string a{std::move(x)}; x += "abc"; }: the PR now reports accessMoved.
• void f12(std::string &r){ std::string s(std::move(r)); r += "x"; }: the PR now reports accessMoved.
• int x; if (b) x = 1; x += 2;, float f; f *= 2.0f;, unsigned u; u ^= a; and int *q; q += 1;: these change from legacyUninitvar to uninitvar.
• void f2(char *p, int n){ if (!p) {} p += n; }: the PR now reports nullPointerArithmeticRedundantCheck. Base already reports the same for char *q = p + n;, so the two forms now agree.

Unchanged:

• Known and symbolic integer values through compound assignment: x = 5; x += 1; 10/(x-6) (zerodiv), int y = x; x += 1; if (y == x), x >>= 1, x *= z and e += n on pointers.
• Container size after s += "a".
• v = {}; v[0] += 1 (containerOutOfBounds).
• q += 0 on a null pointer: no warning.
• p += n after if (!p) return;: no warning.
• A tokenizer loop that does push_back(std::move(cur)); cur = ""; and then cur += c: no false positive.

Message change: for struct Acc { int v; Acc& operator+=(int i){ v = i; return *this; } }; Acc a; a += 1;, base reports Uninitialized variable: a.v and the PR reports Uninitialized variable: a. Both versions warn here, and the operator never reads v, so neither warning is right. Only the reported name changes.

Automated review of 811a54c by Claude. The pull request was built and tested locally.

AI review

I found no new problems. The <<=/>>= gap from the earlier reviews is fixed: vf_analyzers.cpp now checks %assign% with str().size() > 1, which matches every compound operator.

The new isVariableChanged change in astutils.cpp treats a compound assignment as a change of the pointer itself but not of what it points to. I tested it on pointer arithmetic, int** and int*&, iterators, containers and buffers that are filled through an advanced pointer. It added no false positives or false negatives. It removed two false positives that base reports (below), and it found one real uninitialized read.

Earlier comments

Testing

I built cppcheck and testrunner in pr/ and cppcheck in base/.

• ./testrunner -q (all tests): 5418 tests, 0 failures. TestUninitVar, TestValueFlow, TestNullPointer and TestOther also pass when run on their own.

I compared base and PR with cppcheck -q --enable=all --inconclusive --library=std on my files in scratch/.

Results that changed (all improvements):

• int h1() { char buf[10]; char* p = buf; p += 2; return *p; }: the PR now reports uninitvar on *p. That is correct.
• int k3() { char buf[10]; char* p = buf; p += 2; *p = 1; return buf[2]; }: base wrongly reports uninitvar: buf. The PR reports nothing.
• int h5() { int x = 0; int* p = &x; p += 0; *p = 1; return 10 / x; }: base wrongly reports zerodiv. The PR reports nothing.

Results that did not change:

• Pointers: I checked constParameterPointer/constVariablePointer on *p += 1, **p += 1, p[0] += 1, (*p) += 1, t->m += 1, *s += 1 with an overloaded operator+=, int*& r = *p; r += 1, *pp += 1, pp[0] += 1, int*& r; r += 1, t->p += 1 and a[1] += 2 on int* a[]. Base and PR give the same results for all of them.
• Containers: invalidContainer after s += "abc" is still reported for c_str(), data(), begin() and &s[0].
• Buffers filled through an advanced pointer: loops that write *p = 0; p += 1, memset(p, …) and fill(p) give the same results as base.
• accessMoved: it is reported for x += … after a move. There is no false positive after x = "a"; or x.clear();, or when the move is on another path.

Automated review of a2b79cc by Claude. The pull request was built and tested locally.